Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client): force Event Hubs to use V1_0_0_0 #2633

Merged
merged 1 commit into from
Aug 30, 2023
Merged

fix(client): force Event Hubs to use V1_0_0_0 #2633

merged 1 commit into from
Aug 30, 2023

Conversation

dnwe
Copy link
Collaborator

@dnwe dnwe commented Aug 30, 2023

The problem with supporting Azure Event Hubs is that it isn't Apache Kafka under the covers, it's an intermediate proxy that supports a subset of the Kafka APIs at various versions and then maps them onto Event Hubs protocol(s) at the backend. As as result Sarama's current mechanism of specifying the KAFKA_VERSION to determine what protocol versions to support and use doesn't really work properly with Event Hubs because it supports an unusual set of protocols and even defines minimum versions for ProduceRequest (v3) and FetchRequest (v4). For some reason EventHubs is very behind on FetchRequest and MetadataRequest so the max configuration you can use in Sarama is V1_0_0_0 and the minimum is V0_11_0_0.

As we have recently bumped the default Version to V2_1_0_0 and support a wider range of protocol versions, we're more likely to see issues raised by Event Hubs users unless they have already pinned their version correctly. To try and prevent this, attempt to detect use with Event Hubs by inspecting the bootstrap broker addresses and overriding the Version.

Contributes-to: #2470

The problem with supporting Azure Event Hubs is that it isn't Apache
Kafka under the covers, it's an intermediate proxy that supports a
subset of the Kafka APIs at various versions and then maps them onto
Event Hubs protocol(s) at the backend. As as result Sarama's current
mechanism of specifying the KAFKA_VERSION to determine what protocol
versions to support and use doesn't really work properly with Event Hubs
because it supports an unusual set of protocols and even defines minimum
versions for ProduceRequest (v3) and FetchRequest (v4). For some reason
EventHubs is very behind on FetchRequest and MetadataRequest so the max
configuration you can use in Sarama is V1_0_0_0 and the minimum is
V0_11_0_0.

As we have recently bumped the default Version to V2_1_0_0 and support a
wider range of protocol versions, we're more likely to see issues raised
by Event Hubs users unless they have already pinned their version
correctly. To try and prevent this, attempt to detect use with Event
Hubs by inspecting the bootstrap broker addresses and overriding the
Version.

Contributes-to: #2470

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
@dnwe dnwe added the fix label Aug 30, 2023
Copy link
Collaborator

@hindessm hindessm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dnwe dnwe merged commit ae5eee5 into main Aug 30, 2023
@dnwe dnwe deleted the dnwe/event-hubs branch August 30, 2023 11:46
dnwe added a commit to dnwe/components-contrib that referenced this pull request Aug 30, 2023
Pulling in `fix(client): force Event Hubs to use V1_0_0_0` by @dnwe in
IBM/sarama#2633

This should avoid dapr hitting any issues with Azure Event Hubs users
who might not have followed the instructions in dapr/docs#3474 to pin
their version number.

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
dnwe added a commit to dnwe/components-contrib that referenced this pull request Aug 30, 2023
Pulling in `fix(client): force Event Hubs to use V1_0_0_0` by @dnwe in
IBM/sarama#2633

This should avoid dapr hitting any issues with Azure Event Hubs users
who might not have followed the instructions in dapr/docs#3474 to pin
their version number.

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants